Skip to content

Clearly separate local and global directions when using Side#126

Open
dhouck wants to merge 3 commits intoNorth-Western-Development:1.20.1from
dhouck:side-local-global
Open

Clearly separate local and global directions when using Side#126
dhouck wants to merge 3 commits intoNorth-Western-Development:1.20.1from
dhouck:side-local-global

Conversation

@dhouck
Copy link
Copy Markdown

@dhouck dhouck commented Mar 12, 2026

The Side enum can be used for either local directions based on the block FACING, or global cardinal directions. Previously most uses of the enum got the two mixed up, leading to a variety of bugs and inconsistencies. This clearly separates when something is a global Direction, a local direction, or something that could be either.

I found this while working on improving the event support for the Redstone Interface block, following my work on #124. The actual event improvements are a separate pull request I am not quite done with, but this was a pre-requisite, because the events previously gave global directions, which could then not accurately be used in method calls.

dhouck added 2 commits March 12, 2026 04:16
Instead of having different identical variants with different names, use
gson's SerializedName attribute to allow one variant to be referenced
with all relevant names.  Additionally, keep track of which of those
names are relative vs. absolute, so eg. "north" and "back" are no
longer synonyms.  Incidentally, add more top/bottom as extra names for
up/down.

The simplification also applies to RobotOperationSide, but all variants
of that are local so the global/local split does not.  In theory it
could apply to MovementDirection and RotationDirection, but those are
serialized/deserialized as NBT ints so this would break deserialization.

Backwards compatibility:
Despite being in the api package, nobody outside this mod seems to use
the class.  Any VM code that addresses sides by index will be affected,
but the index to side mapping was never intuitive or documented.
HorizontalBlockUtils used minecraft Direction as both global and local
direction, and Side.getDirection was also used as both.  This refactors
all of HorizontalBlockUtils into Side, and more clearly separates global
sides (minecraft Direction), local sides (integer indexes), and
ambiguous (the Side type).  Several bugs with incorrect or missing
global/local conversion were also fixed.

The RedstoneInterface block and card now both support either global or
local sides in their RPC API, and will respond appropriately (ie. a call
to set the NORTH value will set NORTH regardless of orientation, and a
call to set the FRONT value will set the FRONT, which might be any
cardinal direction).  This also works for the bundled interface on the
block.

The ComputerBlockEntity no longer converts to local before getting
capabilities.  Any bus item that should expose computer block
capabilities accepts a global Direction, and must respond as such; it
should have access to the block state and can do the rotation on its own
if necessary.

Backwards compatibility:
Any VM code that uses a global name in a context where it was
interpreted as local, or vice versa, will break; I expect this to be
rare and easy to fix.  I think that's the only functionality change I
wouldn't call a bugfix, but I did not thoroughly test the old behavior
of Project Red: Transmission bundled output so something unexpected
might have changed there.

Further work: The redstone interface card is not up to feature parity
with the block.  Probably it would be best if they shared more code
instead of duplicating most of the functionality for both, to help bring
the card up to parity and avoid future issues.
@dhouck
Copy link
Copy Markdown
Author

dhouck commented Mar 12, 2026

EDIT: I have now tested and confirmed this. Not just for the one commit I noticed earlier, with block detection, but also for the item drop placement.

I have not yet tested but I expect that these changes do not at all interfere with #128. Only one commit there touches on RobotOperationSide and it looks like it shouldnʼt be affected either way by these changes.

@dhouck
Copy link
Copy Markdown
Author

dhouck commented Mar 16, 2026

The last commit does not require #124 except for the generic sense that without it any use of subscriptions without the other PR is very buggy. It does not exacerbate those issues, and if you do manage to use events (with or without #124) it just makes those events a bit more informative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant